Skip to content

Conversation

@bbakernoaa
Copy link
Contributor

@bbakernoaa bbakernoaa commented May 20, 2025

Description

This pull request introduces significant updates to the aerosol configuration and emission handling in the UFS-Aerosols system, streamlining file management, improving modularity, and consolidating configuration files. Key changes include restructuring aerosol configuration files, enhancing biomass burning emissions handling, and updating the chemical input file linking process.

Aerosol Configuration Updates:

  • Refactored config.aero.j2 to organize settings into distinct sections for aerosol and emission configurations, including the addition of new variables like AERO_EMIS_FIRE_VERSION and AERO_EMIS_FIRE_HIST. [1] [2]

Emission Data Management:

  • Consolidated multiple emission configuration files (ExtData.gbbepx, ExtData.qfed, and ExtData.none) into a single file, AERO_ExtData.rc, for better modularity and maintainability. Removed redundant and unused emission entries. [1] [2] [3] [4]

Chemical Input File Handling:

  • Added linking of chemical input files (CHEM_INPUT) in forecast_predet.sh to ensure proper file availability during runtime.

Removal of Legacy Code:

  • Removed the automated generation of AERO_ExtData.rc in parsing_namelists_GOCART.sh, as the new consolidated configuration file eliminates the need for dynamic generation.

Additional Updates:

  • Updated config.com to include new template paths for chemical restart and input files, improving file organization.

Type of change

  • Bug fix (fixes something broken)
  • New feature (adds functionality)
  • Maintenance (code refactor, clean-up, new CI test, etc.)

Change characteristics

  • Is this a breaking change (a change in existing functionality)? NO
  • Does this change require a documentation update? NO
  • Does this change require an update to any of the following submodules? NO
    • EMC verif-global
    • GDAS
    • GFS-utils
    • GSI
    • GSI-monitor
    • GSI-utils
    • UFS-utils
    • UFS-weather-model
    • wxflow

How has this been tested?

  • GCAFS noDA cycled on URSA

Checklist

  • Any dependent changes have been merged and published
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have documented my code, including function, input, and output descriptions
  • My changes generate no new warnings
  • New and existing tests pass with my changes
  • This change is covered by an existing CI test or a new one has been added
  • Any new scripts have been added to the .github/CODEOWNERS file with owners
  • I have made corresponding changes to the system documentation if necessary

@CoryMartin-NOAA CoryMartin-NOAA requested a review from Copilot May 20, 2025 20:40
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request updates aerosol configuration and emissions handling for the UFS-Aerosols system by consolidating file management and enhancing modularity. Key changes include:

  • Removing legacy logic for generating the AERO_ExtData.rc file and consolidating emission configuration files.
  • Adding a link to chemical input files in forecast_predet.sh and updating configuration templates in both aerosol and chemical configuration files.
  • Refactoring configuration files to improve clarity and maintainability.

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
ush/parsing_namelists_GOCART.sh Removed legacy dynamic generation of AERO_ExtData.rc.
ush/forecast_predet.sh Added linking for CHEM_INPUT to ensure chemical input file availability.
parm/ufs/gocart/ExtData.* Removed obsolete emission configuration files, consolidating them into AERO_ExtData.rc.
parm/ufs/gocart/AERO_ExtData.rc Updated emission entries to use chemical input paths and adjusted emission factors.
parm/chem/chem_emission.yaml.j2 Updated templating loops for QFED variables and file copying operations.
dev/parm/config/gfs/config.com Added new chemical template paths.
dev/parm/config/gcafs/config.aero.j2 Reorganized aerosol sections and introduced new aerosol emission settings.

@bbakernoaa bbakernoaa force-pushed the feature/prep_emis_rework branch from 2e06fb3 to b534a8b Compare July 31, 2025 19:47
@bbakernoaa bbakernoaa marked this pull request as ready for review August 7, 2025 15:21
@bbakernoaa bbakernoaa requested a review from aerorahul as a code owner August 7, 2025 15:21
@bbakernoaa bbakernoaa requested a review from lipan-NOAA October 14, 2025 19:59
@bbakernoaa
Copy link
Contributor Author

@aerorahul @DavidHuber-NOAA can we please get this reviewed. It's important for the GCAFS implementation

Copy link
Contributor

@CoryMartin-NOAA CoryMartin-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested an earlier incarnation of this in NRT for several weeks, I think this is good to go for final review and testing

Copy link
Contributor

@DavidHuber-NOAA DavidHuber-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some additional suggestions.

${NLN} "${COMIN_CHEM_INPUT}" "${DATA}/ChemInput"
# Create the ChemInput directory in the local run directory

# if [[ ! -d "${DATA}/ChemInput" ]]; then mkdir -p "${DATA}/ChemInput"; fi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bbakernoaa we should use cpreq here. Linking from COMIN is not allowed (at least not yet) by EE2 standards and we shouldn't be linking whole directories. I'm OK with adding a TODO here if you will create a follow-up issue to tackle this.

@bbakernoaa
Copy link
Contributor Author

@DavidHuber-NOAA I think I took care of all your suggestions and requests

dep_dict = {'type': 'task', 'name': f'{self.run}_stage_ic'}
dependencies.append(rocoto.add_dependency(dep_dict))
dependencies = rocoto.create_dependency(dep_condition='or', dep=dependencies)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bbakernoaa double check these changes, I think it's leading to some issues in dependencies

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

                <dependency>
                        <and>
                                <or>
                                        <taskdep task="gcdas_sfcanl"/>
                                        <taskdep task="gcdas_aeroanlfinal"/>
                                        <taskdep task="gcdas_stage_ic"/>
                                </or>
                                <taskdep task="gcdas_prep_emissions"/>
                        </and>
                </dependency>

I don't think this is correct, we need sfcanl and aeroanlfinal or we need stage_ic

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you explain a little more. It's been running for over a month long simulation at this point

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need it to be

                <dependency>
                        <and>
                                <or>
                                        <and>
                                            <taskdep task="gcdas_sfcanl"/>
                                            <taskdep task="gcdas_aeroanlfinal"/>
                                         </and>
                                        <taskdep task="gcdas_stage_ic"/>
                                </or>
                                <taskdep task="gcdas_prep_emissions"/>
                        </and>
                </dependency>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't this mean that its (gcafs_prep_emissions) & ( gcdas_sfc_anl | gcdas_aeroanlfinal | gcdas_stage_ic)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so what you are saying is that we need

(gcafs_prep_emissions) & ( (gcdas_sfc_anl & gcdas_aeroanlfinal) | gcdas_stage_ic)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok i can make that change

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bbakernoaa yes, so we need both sfcanl and aeroanlfinal, not one of them, to be done before the forecast can start

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants